T055 follow-up: confine to the resolved entry, no second lookup (default_registry.resolve_source) - #70
Conversation
…ult_registry.resolve_source) Copilot at openDox-code#59 0c946f4 ("previously missed", medium, default_registry.py:444): resolve_source() performs a second lookup by (repository, ref) after the caller has already resolved an entry, so a concurrent refresh can put another entry's source_root behind the path. serve_project._resolved_listed_edit_entry, the only production caller of that two-step path, resolved an entry and then asked the registry for the path by the entry's pair. It now resolves once and confines the path to THAT entry's own root through the registry seam's declared resolve_within, as serve.py's /source arm does since #59. The listed-path check and the editor launch already used the entry in hand, so lookup, validation and confinement are one entry. The route no longer needs a method the seam does not declare, so a contributed registry without resolve_source serves it. SnapshotRegistry.resolve_source keeps its behaviour (one lookup, confined to that entry) and its docstring now says it is for a caller that holds only a pair. tests/test_edit_action_one_entry.py holds the rule: one lookup, both directions of the race at the route, confinement kept, a host registry, and no module asking a registry for a path by a pair. Arc: neutral-product-standalone-operability Lane: openxfactory-4 (openXfactory-4-openDox_extraction) Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Reviewer's GuideThe PR removes the remaining unstable second registry lookup from the project edit route by confining paths through the entry returned by its single resolution, while retaining existing listing and containment rules. It documents the intended scope of Sequence diagram for single-entry project edit path resolutionsequenceDiagram
participant Client
participant EditRoute as serve_project
participant Registry
participant Seam as projection_seams.registry
participant Entry
participant Editor
Client->>EditRoute: POST /actions/edit
EditRoute->>Registry: resolve(repository, ref)
Registry-->>EditRoute: Entry
EditRoute->>Entry: read listed source paths
EditRoute->>Seam: resolve_within(Path(Entry.source_root), path)
Seam-->>EditRoute: confined target
alt target is a listed file
EditRoute->>Editor: launch editor over Entry.source_root
EditRoute-->>Client: 200
else target unavailable or unlisted
EditRoute-->>Client: 404 document_unavailable
end
Sequence diagram for refresh-safe source confinementsequenceDiagram
participant EditRoute as serve_project
participant Registry
participant Seam as projection_seams.registry
participant FirstEntry as resolved entry
participant Replacement as refreshed entry
EditRoute->>Registry: resolve(repository, ref)
Registry-->>EditRoute: FirstEntry
Note over Registry,Replacement: A refresh may replace the key after resolution
EditRoute->>Seam: resolve_within(Path(FirstEntry.source_root), path)
Seam-->>EditRoute: target confined to FirstEntry
EditRoute->>FirstEntry: read listed source paths
EditRoute-->>Replacement: no lookup by repository and ref
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Copilot review overview
🟢 Approval recommended
The focused change correctly removes the second lookup and is comprehensively covered.
Review effort: Balanced
Findings: None
What changed in this PR
Fixes a registry-refresh race by confining edit paths to the already-resolved snapshot entry.
Changes:
- Uses one stable registry entry throughout edit validation.
- Documents
resolve_source’s pair-only contract. - Adds focused confinement and race coverage.
| File | Description |
|---|---|
src/opendox/serve_project.py |
Confines edits to the resolved entry’s root. |
src/opendox/default_registry.py |
Clarifies lookup semantics. |
tests/test_edit_action_one_entry.py |
Tests single lookup, confinement, host seams, and refresh races. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
…r, do not copy them SonarCloud's quality gate on this PR read 4.9% duplication on new code (required at most 3%): the new test module carried a copy of test_projection_seams.py's autouse registry-isolation fixture and its git helper. Import both instead, as tests/test_doxbench_*.py import their shared helpers from test_doxbench_view. The autouse fixture applies to the new module's cases (eight SETUPs under --setup-show), the eight cases pass, and all five mutants of the fix are still killed. Arc: neutral-product-standalone-operability Lane: openxfactory-4 (openXfactory-4-openDox_extraction) Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
|
|
READY at 5666505 — Lane: openxfactory-4 (openXfactory-4-openDox_extraction) |
There was a problem hiding this comment.
Sorry @brettheap, you've used your own review budget of 250,000 diff characters for the last 7 days.
You can request another review in 1 day and 20 hours by commenting @sourcery-ai review. Upgrade to get a review now.
main now carries #70 (75bd8705, resolve_source confined to the resolved entry) and #66's squash (a23e422). This branch already held #66's final head, a7bda06, at 923f30d9. So main brings only #70's three files (default_registry.py, serve_project.py and tests/test_edit_action_one_entry.py), and T058 touches none of them. The merge is clean. Proof that the merge carries exactly T058's delta: the stable patch-id of `git diff a7bda06 923f30d9` equals the patch-id of `git diff origin/main` against this merge. That delta is seven files. Lane: openxfactory-4 (openXfactory-4-openDox_extraction) Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…n 034) (#68) Lane: openxfactory-4 (openXfactory-4-openDox_extraction) Plan 034 **T058** [US2] [oDc], **the post-render validator in the generate verbs** (#1144's 7.2, in part), from `specs/034-opendox-standalone-operation/tasks.md` at openxFactory `main` `91e4685f`: > It validates the neutral snapshot against T053's schema, read from T057's packaged copy. `--strict` makes a validator that cannot run fatal, and `--no-validate` skips validation. > - **Realizes**: 7.2 (part). > - **Falsifier**: F7.2. The good fixture exits 0; the malformed one exits non-zero, naming `EXPECTED_RULE`, with no `No such file or directory`. > - **Ruled**: R1Q22 (a), `5817152735`; R1Q11 (a), R1Q12 (a), `5850003126`. > - **After**: T051, T056, T057. Claimed on openxFactory#656 in comment `5901343950`, with T056. ## Was stacked on #66, and is now on `main`, with its predecessors landed - **It was stacked on T056's branch** (#66), which was itself stacked on #59 (T055). Each new #66 head was merged in here, the last being `a7bda066` at `923f30d8`. - **#66 has landed** as `a23e4224`, after #70 (`75bd8703`, the `resolve_source` follow-up to #59). `main` is merged in at `5322efee`. It brought only #70's three files, `default_registry.py`, `serve_project.py` and `tests/test_edit_action_one_entry.py`, and T058 touches none of them. - **The merge carries exactly T058's delta.** The stable patch-id of `git diff a7bda06 923f30d` equals the patch-id of `git diff main 5322efe`: `f302a8aa` both times. The same id held at `cd6b33cb` against `38761c76`. - **#58 (T057) was merged in**, at `e94ab323`, `3351f6a7` and `753ffa19`, because T058 wires the `opendox.validator` that #58 ships. **#58 has since landed** as `8ec08e91`, which reaches this branch through #59 and #66 (`03825eb5`, no content change: T057's files here equal `main`'s byte for byte). - So this PR's diff is now **T058's own seven files**. Until #58 landed, it also showed #58's delta. - **T058's own changes are six commits**: `3d0b6d66` (six files, listed below), and `28241a4b`, `69ca0e2e`, `1ec7d2c9`, `c7768ed5` and `cd6b33cb` (Copilot's findings, below). - #58 and #59 fork from #57 at the same commit, `8e7da4a2`, so the merge was clean and brought only #58's own 55 files. - **Gated on nothing but its own review.** #57 (`a691e4e4`), #58 (`8ec08e91`), #59 (`fa140875`) and #66 (`a23e4224`) have all landed. - **The base is `main`.** It was retargeted before #66 landed, since a `--delete-branch` landing of #66 would have closed it. The diff against `main` is T058's own seven files again. ## What T058 changes **`src/opendox/default_projection.py`: openDox's own validator takes the stand-in's place.** The stand-in, `OwnValidatorNotBuilt`, answered every validation "unavailable, T057 is not here". - **One adapter per own kind.** `OwnValidator(kind)` keeps the lookup's protocol, `validate(path, *, strict, search_from)` → `projection_seams.ValidationResult`. - The seam registers one validator per kind and hands it only a path. So the adapter knows its kind from where it is registered. - `VALIDATORS` holds one for each of `OWN_KINDS`: `opendox-snapshot` and `ideation-workbench`. - `projection_seams.register_defaults()` registers each under its kind (a one-line change). - `OWN_KINDS` is not widened. The doxBench wire kinds reach `opendox.validator` through their own seam (T085). - **It reads the document as its kind is written.** - The snapshot is read as JSON, strictly: NaN, the infinities and a key given twice are refused. - The manifest is read as YAML with `safe_load`, as `workbench.py` reads it. - Then `opendox.validator.validator_for(kind)` judges the document against the packaged copy, which `opendox.contracts` proves against its recorded digest on every call. - **Three outcomes**, which `cli._validate` already turns into consequences: | the adapter meets | outcome | what the verb does | |---|---|---| | no violation | `validated` | prints `validation: opendox-snapshot: 0 violations, by opendox.validator, over its packaged copy opendox-snapshot (sha256 f9e3e111af1d)`, exit 0 | | any violation | `not-conformant`, rc 1; stdout is one `[<rule>] <where>: <detail>` line per violation, then a count | "validation FAILED … This is the SNAPSHOT", relays the lines on **stderr**, exit 1 (with or without `--strict`) | | a document that cannot be read as JSON (or YAML) | `not-conformant`, rule `document-syntax` | as above | | a readable document holding a number that cannot be read as written (an infinity, a NaN, one binary64 would round, or an unprovable spelling) | `not-conformant`, rule `document-number` | as above | | `ValidatorUnavailable` (a copy failing its identity check, or not evaluable), `UnknownKind`, or a document that cannot be read | `validator-unavailable`, with the reason | "validation SKIPPED … could not run: <reason>", exit 0; **exit 1 under `--strict`** | - **`strict` and `search_from` change nothing** in the adapter. openDox's validator has no warnings to harden, and it searches for nothing, since its schemas are package data. `dependency_remedy` is `None`: no subprocess runs. - So F7.2's "no `No such file or directory`" holds by construction. - `--strict` still means what the verb's help says, through `cli._validate`. **The workbench manifest's two validator rules, carried (for the holder).** - The manifest schema says of two rules that it cannot state them, and leaves them to the validator: - every `recipe.pinned` keyword is also in `recipe.checked`; - no `recipe.new_candidates` document is already a member or excluded. - The consumer's script (`validate-ideation-dashboard-contracts.py`, `check_workbench_rules`) checked both. `opendox.validator` checks the schema only, as #58's body says. - The holder decided (2026-09-28, "To T055: carry two workbench rules") that routing `validate_manifest` to openDox's validator must not drop them. T055 routed it to the stand-in, so they fall due here. - The adapter carries them for `ideation-workbench`, under the script's identifiers, `workbench-pinned-not-checked` and `workbench-candidate-overlap`. They are judged beside the schema, and are total over any shape. - If the holder prefers them in `opendox.validator` itself, that is a move within #58's module, and the ids stay. **The schema copy.** - `tests/test_neutral_projection.py` reads the neutral contract from the packaged copy, through `opendox.contracts.verified_bytes("opendox-snapshot")`. - T054's `tests/fixtures/opendox-snapshot.schema.yaml` and its `SCHEMA_SHA256` are removed. The bytes were identical (`f9e3e111…584a` both), so no case's verdict moved. - The digest case now asserts that the bytes read are the recorded ones, that a strict JSON read equals `contracts.load()`'s YAML read, and that the tree carries one copy. **The stand-in's cases in `tests/test_projection_seams.py`**, as T055's hand-off listed them: | before | after | |---|---| | `test_the_validator_stand_in_concludes_nothing_and_names_T057` | `test_openDoxs_own_validator_is_bound_to_each_own_kind` | | `test_openDoxs_own_kind_meets_the_stand_in_and_strict_makes_it_fatal` | `test_openDoxs_own_kind_meets_openDoxs_own_validator` (a bare snapshot is now REJECTED, naming `[envelope-keys]`), and `test_openDoxs_own_validator_unavailable_is_skipped_and_strict_makes_it_fatal` (a copy refused by `opendox.contracts`: skipped, then fatal under `--strict`) | | the stand-in half of `test_a_manifest_is_validated_by_the_validator_for_its_kind` | a bare manifest is now judged, naming `[required]` | Two identity asserts also move from `default_projection.VALIDATOR` to `VALIDATORS[kind]`. **`tests/test_post_render_validator.py` (new, 29 cases):** - F7.2 through `python -m opendox.cli generate --strict`, with the siblings refused by T056's `tests/standalone_child.py`; - `generate-and-open --no-open --no-serve --strict` over both fixtures; - `--no-validate`; - the three outcomes, including five not-JSON documents, an unreadable path, `ValidatorUnavailable`/`SchemaNotEvaluable`, and a copy tampered below `opendox.contracts`; - `strict`/`search_from` inert; - each validator reading as its own kind; - the two workbench rules, their bounded detail, and `workbench.save(validate=True)` keeping a valid manifest and unwinding a broken one. ## F7.2, failing before and passing after **#1144's F7.2, verbatim**: a fresh venv, `pip install .` (package data on disk, a non-editable install), both fixtures as fresh repositories, `generate --strict` twice, the rule grep, and the negative grep asserted as exit status 1. - **Before**, at this branch's base (`3b13f141`: #59 + #66 + #58, with the stand-in), it exits **1** on the good fixture's `--strict` run: ``` validation SKIPPED — this snapshot was NOT checked against the pinned schema the validator registered for kind 'opendox-snapshot' reached no verdict. … openDox's own validator is plan 034's T057, and this build does not carry it yet, so nothing of openDox's own kinds is checked --strict was given and it means what it says: a run that COULD NOT be validated FAILS rather than continuing unchecked ``` - **After**, at `cd6b33cb`, and again at `5322efee`, after `main` was merged in, it exits **0**. The malformed run's stderr: ``` validation FAILED — the pinned validator REJECTED …/bad.json. This is the SNAPSHOT, not the environment: the validator ran fine and found the data non-conformant. [title-and-summary-are-text] /documents/1/title: '' is shorter than 1 1 violation(s) of the opendox-snapshot contract, by opendox.validator, over its packaged copy opendox-snapshot (sha256 f9e3e111af1d) ``` `EXPECTED_RULE` is `title-and-summary-are-text`, and no `No such file or directory` appears. **`tests/test_post_render_validator.py`**: at the base, with the stand-in, **26 failed and 2 passed**. The two that pass hold what T058 does not change: `--no-validate`, and `EXPECTED_RULE` being one of the contract's rules. At `3d0b6d66`, **29 passed**. At this head `1678ccd0`, with Copilot's later rounds, **50 passed**, and also 50 under `--noconftest`. ## Mutation check: 29 of 29 killed, re-run at this head Each mutant was applied, the named cases were run, and the sources were restored and checked by sha256. | mutant | killed by | |---|---| | M1 violations answer `validated` | 17 cases | | M2 violations answer `unavailable` | 12 | | M3 the rule lines are dropped from stdout | 17 | | M4 `ValidatorUnavailable` read as a pass | 4, including the `--strict` case | | M5 NaN admitted as JSON | 1 | | M6 a repeated key admitted | 1 | | M7 the pinned rule dropped | 4 | | M8 the overlap rule dropped | 1 | | M9 the overlap ignores `excluded` | 1 | | M10 the validator reads the document's `kind`, not its own | 1 | | M11 the entry points register only the snapshot's validator | 2 | | M12 the rules compare unhashable entries | 1 | | M13 an unreadable document reads as valid | 1 | | M14 `--strict` does not make an unavailable validator fatal (`cli.py`) | 1 | | M15 `--no-validate` does not skip (`cli.py`) | 4 | | M16 T054's fixture copy of the schema comes back (a tree mutant) | 1 | | M17 a rule's detail quotes every name | 1 | | M18 name membership tested against the list again (quadratic) | the linear-time case, at 17.0 s | | M19 an `OSError` from the lookup escapes | the unreadable-file case | | M20 a quoted name is not cut | the long-name case | | M21 a key given twice picks the last `kind` again (`cli.py`) | the doubled-kind case | | M22 a non-finite number is read as JSON | the `1e999` and YAML `.inf`/`.nan` cases | | M23 a rounded number is read as written | the `1.0000000000000001` and `1.5e-400` cases | | M24 every inexact binary fraction refused (over-strict) | the controls (`0.1`, `2.50`, …) | | M25 the manifest's floats are read unproved | the four manifest-number cases | | M26 the JSON read proves no float | the snapshot number cases | | M27 an unprovable number spelling keeps its float | the two base-60 manifest cases | | M28 a refused number is reported as a syntax error | the number-rule cases | | M29 the syntax message names the kind as an adjective again ("a 'opendox-snapshot' document") | the two exact-message syntax cases (6 failed) | ## The repository's own checks The whole suite ran locally as CI runs it: `CI=true`, `LANG=C.UTF-8`, PostgreSQL 16, `-e ".[runtime,test]"` with the constraints file, at the committed head, with a clean tree. | tree | passed | skipped | |---|---|---| | base `3b13f141` | 2948 | 11 | | `3d0b6d66` (T058's commit) | 2978 | 11 | | `09cd1e8a` (#66's `42a08a31` merged in) | 2979 | 11 | | `28241a4b` (Copilot's two findings) | 2981 | 11 | | `21e4723f` (#66's `5a26532e` and #58's `3351f6a7` merged in) | 3010 | 11 | | `69ca0e2e` (#66's `e939c31f` merged in, and Copilot's second round) | 3014 | 11 | | `80153754` (Copilot's third round, and #58's `753ffa19` merged in) | 3031 | 11 | | `c7768ed5` (#66's `e3574774` merged in, and Copilot's fourth round) | 3033 | 11 | | `cd6b33cb` (#66's `38761c76` merged in, and Copilot's fifth round) | 3034 | 11 | | `923f30d8` (#66's final head `a7bda066` merged in), run under `nohup … &` | 3036 | 11 | | `5322efee` (`main` merged in, with #66 and #70 landed), run under `nohup … &` | 3044 | 11 | | this head `1678ccd0` (Copilot's sixth round: the syntax message's wording), run under `nohup … &` | **3044** | **11** | - The junit diff at `3d0b6d66` shows **+32 added** (29 in the new file, 3 in `test_projection_seams.py`) and **2 removed** (the two stand-in cases above, replaced). At `09cd1e8a` it shows +33: the extra case is #66's own. - At `923f30d8`, against `cd6b33cb`, it shows **+2**: #66's `test_a_child_stops_on_the_interrupt_even_when_the_runner_ignores_it` and #59's `test_a_regenerate_promotes_no_session_and_moves_no_active_key`. At `5322efee`, against `923f30d8`, it shows **+8**, all from #70's `tests/test_edit_action_one_entry.py`. At this head, against `5322efee`, it shows 0 added, 0 removed and 0 changed. - **0 changed** outcomes, and the same 11 skips. - The mutation check was re-run at `923f30d8` and `5322efee` (28 of 28 killed both times), and at this head, where it is **29 of 29**. - **F4.1's deferred-reach scan**: 11 at the base and 11 here, the same list. The adapter imports `opendox.validator` and PyYAML when a validation runs, and names no sibling. - No floor, workflow, `conftest.py`, `pyproject.toml` or pin is touched. ## Files (T058's own commits) - `src/opendox/default_projection.py`: `OwnValidator`, `VALIDATORS`, `SYNTAX_RULE`, `NUMBER_RULE`, `WORKBENCH_RULES`. The stand-in is removed and the docstring rewritten. - `src/opendox/projection_seams.py`: `register_defaults()` registers `VALIDATORS[kind]`. - `src/opendox/cli.py` (`69ca0e2e`): `_written_kind` refuses a key given twice, and `_validate` fails on it. These seven files are the whole of the PR's diff now that #58 has landed. - `tests/test_projection_seams.py`: the stand-in cases above. - `tests/test_neutral_projection.py`: it reads the packaged copy. - `tests/fixtures/opendox-snapshot.schema.yaml`: removed. - `tests/test_post_render_validator.py`: new. It is a created file, with no carve-manifest row (RULED OQ-C). ## Copilot - At `3d0b6d66`, "Needs a closer look", with two findings. Both are fixed in `28241a4b`, answered with evidence, and resolved: - **r4139734412**: an unreadable packaged record or copy escaped `validator_for()` as a `PermissionError` traceback. `opendox.contracts` converts only a missing file. - Measured with `copies.yaml` at mode 000: `generate --strict` exited 1 with the traceback. - The adapter now reports an `OSError` from the lookup as validator-unavailable, and the verb warns, or fails under `--strict`, in its own words. - The root conversion is #58's file, and it has been relayed to #58's owner. - New case: `test_a_packaged_file_that_cannot_be_read_is_unavailable_not_a_traceback`. - **r4139734444**: `_names()` was quadratic, and the schema bounds none of the three lists. Membership is now a set's. New case: `test_the_rules_read_a_long_list_in_linear_time`, which took 17.5 s before the fix and 0.01 s after. - At `21e4723f`, "Needs a closer look", with four findings, each answered with evidence and resolved: - **r4139769819**: the verb chose a validator by `kind` with plain `json.loads`, which keeps the last of two keys. So `"kind": "opendox-snapshot", "kind": "unknown"` found no validator, and an ordinary run exited 0. - Fixed in `69ca0e2e`: `cli._written_kind` refuses a key given twice, and the verb fails whatever `--strict` says. New case: `test_a_kind_given_twice_chooses_no_validator_and_fails`. - NaN and the infinities are left to the chosen validator, since they do not make the kind ambiguous. - **r4139840593**: `1e999` reads as `inf` without `parse_constant`. Fixed in `69ca0e2e` (`parse_float=_finite`), with two new not-JSON cases. - **r4139769791**: a quoted name was not cut. Fixed in `69ca0e2e`: each is cut at 80 characters. New case: `test_a_rules_detail_cuts_a_long_name`. - **r4139769759** (`validator.py`, #58's file): `_close` consuming earlier siblings' canons at `count == 0`. **Not reproduced**: the slice is `done[len(done) - count:]`, which is empty at 0, and `_canon([1, []])` and `_canon([[], 1])` are distinct (`a2:n1:1a0:`, `a2:a0:n1:1`). No change here; the note went to #58's owner. - (r4139734412's root, `opendox.contracts` refusing an unreadable file, has since landed in #58 as `2b8ad24`, merged in here. With it, a record at mode 000 gives `could not run: opendox.contracts has copies.yaml, and it cannot be read (PermissionError: Permission denied)` and no traceback. The adapter's own `OSError` guard stays as defense in depth.) - At `69ca0e2e`, one finding, answered with evidence and resolved: - **r4139937566**: `float()` rounds `1.0000000000000001` to `1.0`, which meets `const: 1`. A manifest so written read as "0 violations", and jsonschema 4.26 reads the snapshot case the same way. - The contract has no `number` type, so rather than carry decimals through #58's validator, `1ec7d2c9` refuses a float literal that is not finite, or not equal to the shortest spelling of the float read from it, under `document-syntax`. The same proof applies to the manifest's YAML floats. - `0.1`, `2.50`, `1E2` and every float openDox's writer writes read as written. - Six new cases fail without the fix, and seven controls pass either way. - At `80153754`, one finding, answered with evidence and resolved: - **r4146201125**: a YAML base-60 float (`0:1.0000000000000001`) is read and rounded by PyYAML, but `Decimal` cannot parse it, so the proof let it through, and the manifest read as "0 violations". - Fixed in `c7768ed5`: a spelling the proof cannot compare is refused, under `document-syntax`. - Two new cases fail without the fix, and mutant M27 is killed. - At `c7768ed5`, one finding, answered with evidence and resolved: - **r4146428769**: a valid document refused for a number was reported as "cannot be read as YAML/JSON". - Fixed in `cd6b33cb`: such a number breaks a rule of its own, `document-number`. `document-syntax` stays for a document that cannot be read at all. - Ten cases fail without the change, and mutant M28 is killed. - At `cd6b33cb`, "Needs a closer look", with nothing open. - At `5322efee`, "Needs a closer look", with one finding, answered with evidence and resolved: - **r4148179148**: `document-syntax`'s message read "a 'opendox-snapshot' document", which takes the wrong article. - Fixed in `1678ccd0`: it now reads "a document of kind 'opendox-snapshot'". - The two exact-message cases, updated first, failed against the old wording (6 of 6 runs), and mutant M29 is killed. - At this head `1678ccd0`, a review is re-requested through the reviewer API. ## For the holder 1. **The two workbench rules** (above): carried in the adapter, under the consumer script's ids. Tell me if they belong in `opendox.validator` instead. 2. **The verb relays at most the last 20 lines** of a rejection (`cli._report_non_conformance`, T055's, unchanged). A snapshot breaking more than 19 rules shows the count line and the last 19. That is enough for F7.2's single rule, and not changed here. Arc: neutral-product-standalone-operability Lane: openxfactory-4 (openXfactory-4-openDox_extraction) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Arc: neutral-product-standalone-operability Lane: openxfactory-4 (openXfactory-4-openDox_extraction) Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Main now carries T054 to T058, T055's follow-up (#70) and T056's standalone test. This PR edits src/opendox/runtime/config.py and tests_runtime/test_runtime_cli.py, and main touches neither, so the merge is clean. Full suite on the merged tree: 3061 selected, 3050 passed, 11 skipped, 0 failed. EXPECT_SKIPPED=11 holds exactly, and the floors are met. Lane: openxfactory-4 (openXfactory-4-openDox_extraction) Arc: neutral-product-standalone-operability Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
T071 (#60) now carries main 047bb4f: phase 2, with T054 to T058, T055's follow-up #70 and T056's standalone test. Git auto-merges cli.py and test_doxbench_entrypoint.py without a conflict: main's _refuse_empty_source_options sits after the install shape is resolved, and --local still precedes --host. Four callers on main relied on generate-and-open's old default, and since this PR an unflagged run is HOSTED and refuses without its broker's issuer. They get --local in the next commit, which T070 owes now that T056 has landed. Lane: openxfactory-4 (openXfactory-4-openDox_extraction) Arc: neutral-product-standalone-operability Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
T070 (#67) now carries T071's merge of main 047bb4f: phase 2, with T054 to T058, T055's follow-up #70 and T056's standalone test. It also carries the four generate-and-open callers that now say --local. Git auto-merges pyproject.toml (main's validator package data beside this PR's local extra and data files), src/opendox/cli.py and tests/test_doxbench_entrypoint.py without a conflict. On their own, the merged callers run --local, and here that starts the bundled server. Three of them would do so under the user's own state directory. The stand-in driver no longer stands in for anything, so three bundled cases fail on this merge alone. The next commit takes both in hand. Lane: openxfactory-4 (openXfactory-4-openDox_extraction) Arc: neutral-product-standalone-operability Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
… collapsed DSN pair (plan 034) (#60) Lane: openxfactory-4 (openXfactory-4-openDox_extraction) Plan 034 (`specs/034-opendox-standalone-operation/tasks.md`, read at openxFactory `main` `e369cb25`), phase-3 slice **P3-I, install mode and the bundle**, its first task only: - **T071** (#1144's 13.2 and 13.3). `load_settings` refuses a non-PostgreSQL DSN, naming the one dialect kept. It refuses the same credential in both settings, naming `OPENDOX_MIGRATION_DATABASE_URL`. **`OPENDOX_MIGRATION_DATABASE_URL` is required only for the `migrate` path** (RULED, see below) — `load_settings` (served/status/init/project-verb) keeps it optional; `load_migration_settings` (migrate/reset) already required it, independently of this PR. Falsifier: F13.1's `load_settings` block. After: T063, T069 (both done: T063 landed as openxFactory#1218 → `a883bbf6`). Claimed on openxFactory#656 in [`5876416600`](opensoft/openxFactory#656 (comment)). **Authored ahead under the phase-3 draft-ahead rule** (Brett, 2026-09-28 ~18:00Z: *"Only the independent ones (Recommended)"*). Only T071 and T078–T080 (P3-B, a separate stacked PR) are drafted before T063. This PR stayed DRAFT until T063 had landed and the holder said so. **T063 has landed** (openxFactory#1218 → `a883bbf6`, 2026-10-02 23:37:43Z), which closes phase 2, so phase 3 is open and this PR is the first of its stack to go READY. **Not T070.** 13.4–13.6 (`OPENDOX_INSTALL_MODE`, and the `--local` flag) are untouched — T071's own `After:` line names T063 and T069 only, and nothing here reads or names `OPENDOX_INSTALL_MODE`. 13.2/13.3 build cleanly without it. ## What it does - **13.2 — the dialect.** `_refuse_non_postgresql_dsn` (new) refuses either DSN whose URI scheme is not `postgresql://` or `postgres://`, naming the setting and the dialect found. The keyword/value conninfo form (`host=h dbname=d …`) names no dialect at all and is unaffected — that syntax is libpq's own grammar, and no other driver reads it. A DSN `urlsplit` itself cannot parse (an unbracketed IPv6 host, measured) is a named `ConfigurationError`, never a bare exception — the same shape `_split_url` already covers for the broker settings. This check is a no-op on an ABSENT migration DSN — see 13.3. - **13.3 — the collapse is refused; the migration DSN is required only for `migrate`.** - `OPENDOX_MIGRATION_DATABASE_URL` stays **optional** in `load_settings` (`Setting`'s `required` flag is `False`, `_optional` reads it, `RuntimeSettings.migration_database_url` keeps `str | None`) — RULED "Required only for migrate" (Brett, openxFactory#656, on the claim thread for plan 034's T071, 2026-09-28; see "Resolved by ruling" below). It is never defaulted from `OPENDOX_DATABASE_URL`. `load_migration_settings` (the `migrate`/`reset` loader) is unaffected either way: it already independently required one. - `_refuse_the_same_dsn_in_both_settings` (new) refuses the two DSNs being the exact same STRING, naming `OPENDOX_MIGRATION_DATABASE_URL` — asked only once the two are already known to **agree** on where they land (the existing `_refuse_two_dsns_that_select_different_schemas`, unchanged, now runs first): two DIFFERENT secrets for one role still pass, exactly as the existing "single-role install" case (`test_a_dsn_that_names_no_database_still_reaches_one`) documents. - Both `_refuse_non_postgresql_dsn` and `_refuse_the_same_dsn_in_both_settings` are **no-ops when the migration DSN is absent** (the same shape `_refuse_two_dsns_that_select_different_schemas` already used for "nothing to compare"), and run exactly as before — dialect → schema-mismatch → collapse — whenever both DSNs are actually given. Optional does not mean unchecked. ## The ripple (opened, then reverted, by the ruling) An earlier version of this PR (`91f7973`) made the migration DSN required, which broke 27 existing call sites across five test files. The ruling above keeps it optional, so that ripple has been **reverted in full**, back to `main`: - `tests_runtime/conftest.py`, `test_api_endpoints.py`, `test_migrations_apply.py` and `test_runtime_surface.py` are byte-for-byte `main` again — no `migration_dsn` fixture, no threading, nothing. - `test_runtime_cli.py`'s 20 touched call sites (individual `monkeypatch.setenv` blocks, `base`/`env` dicts, the shared `_stub_uvicorn` helper) are reverted the same way. - `test_two_dsns_that_select_different_schemas_are_refused`'s "AND A MIGRATION DSN THAT IS SIMPLY ABSENT" case is back to **accepted** (`assert load_settings(...)`), which is what the setting being optional again means for that test — with a one-line note on why it was briefly the opposite. - **Six tests now cover 13.2/13.3 directly**, three unchanged from before plus three new ones: `test_a_non_postgresql_dsn_is_refused_naming_the_dialect_kept`, `test_an_unparseable_dsn_is_refused_and_never_raises_a_bare_valueerror`, `test_the_same_dsn_in_both_settings_is_refused_naming_the_migration_one` (all three test `load_settings` directly, both DSNs always given), plus `test_serve_and_status_load_with_no_migration_dsn_configured` and `test_the_collapse_is_refused_through_the_served_workload_too` (both at the CLI dispatch level — `status`/`serve` — rather than only through `load_settings` called directly), plus `test_migrate_refuses_a_non_postgresql_migration_dsn_at_configuration` (`f097fd8`, a follow-up Copilot review round — see below). `test_migrate_refuses_rather_than_borrowing_the_served_identity` (pre-existing, untouched) already covered "migrate refuses without it." ## Fix round: `load_migration_settings` gets the same dialect gate (`f097fd8`) Copilot's re-review of `b5296f9` found that 13.2's dialect gate was wired into `load_settings` only. `load_migration_settings` — the loader `runtime migrate`/`reset` actually use — checked only that the migration DSN was non-empty and handed it straight to `Database`, so a non-PostgreSQL migration DSN reached the driver instead of being refused by name at configuration: the same un-named failure 13.2 exists to prevent for the served loader, reachable through the one path F13.1's own falsifier does not call. Fixed with one more call to the existing `_refuse_non_postgresql_dsn`; `test_migrate_refuses_a_non_postgresql_migration_dsn_at_configuration` is the dialect-refused twin of the existing `test_migrate_and_reset_need_no_served_identity_and_no_broker` (which shows an unreachable-but-valid-dialect migration DSN getting PAST configuration). [Finding](#60 (comment)) and [reply](#60 (comment)), thread resolved. A second finding from the same round ([here](#60 (comment))) read a stale, pre-ruling snapshot of this body ("required" language) — by the time it posted, the body already said "optional… required only for migrate" throughout. [Replied](#60 (comment)) noting the race and inviting a quote of any sentence still reading that way; thread resolved. ## The falsifier (F13.1's `load_settings` block) Quoted from `openspec/changes/add-neutral-product-standalone-operability/tasks.md`, run against this branch: ``` from opendox.runtime.config import ConfigurationError, load_settings OK = {"OPENDOX_OIDC_ISSUER": "https://issuer.example.invalid/realms/fixture", "OPENDOX_OIDC_AUDIENCE": "fixture"} def refusal(**dsns): try: load_settings({**OK, **dsns}) except ConfigurationError as e: return str(e) raise AssertionError(f"accepted: {dsns}") m = refusal(OPENDOX_DATABASE_URL="sqlite:///x.db", OPENDOX_MIGRATION_DATABASE_URL="sqlite:///x.db") assert "postgres" in m.lower(), f"a second dialect was refused for the wrong reason: {m}" one = "postgresql://one@127.0.0.1/opendox" m = refusal(OPENDOX_DATABASE_URL=one, OPENDOX_MIGRATION_DATABASE_URL=one) assert "OPENDOX_MIGRATION_DATABASE_URL" in m, f"a collapsed pair was refused for the wrong reason: {m}" ``` Both assertions pass on this branch (verified interactively; both calls give both DSNs, so the ruling above does not change either outcome; T074 is the task that runs F13.1 whole, later in Group 13, after T072/T073 exist). ## The repo's own suite Measured locally against this branch: this sandbox's host-mapped loopback ports are unreachable (a local environment quirk, not this change — even a fresh container's freshly-published port refuses a host-side connection here), so Postgres is reached at the container's own bridge IP instead of `127.0.0.1`, which is otherwise identical to `validate.yml`'s own DSN shape. ``` python -m pytest -q 2473 passed, 11 skipped, 1 failed in ~195s ``` The one failure, `tests/test_model_provider_broker.py::test_the_broker_child_inherits_no_credential_shaped_environment`, is **pre-existing**: it is red the same way against unmodified `main` (`2d116415`) in this same sandbox (an ambient `LC_CTYPE` this environment sets and the check's allowlist does not name) — unrelated to `runtime/config.py`, and not touched by this PR. It does not reproduce in this PR's own CI (below), confirming the sandbox-only attribution. Against `main`'s own last recorded CI triple (2479 selected / 11 skipped, `validate.yml`'s own comment history) this change is **+6 / +6 / +0** — six tests (three carried over, three new: two for the ruling, one for the follow-up fix round below), nothing lost, the original ripple fully reverted. `validate.yml`'s `Pin the triple` floors (`MIN_SELECTED=2476`, `MIN_PASSED=2465`, `EXPECT_SKIPPED=11`) permit the rise unchanged; I have not touched them. **This PR's own CI (`f097fd8`): `validate` and `SonarCloud Code Analysis` both green.** ## Merge-from-main round (`adeb6fed`, 2026-10-02) Phase 2 has landed, so this branch merges main `047bb4fa`: T054 to T058, T055's follow-up #70 and T056's standalone test. - **The merge is clean.** This PR edits `src/opendox/runtime/config.py` and `tests_runtime/test_runtime_cli.py`, and main touches neither. - **Full suite on the merged tree**, run with `LANG=C.UTF-8` and `CI=true` against a `postgres:16`: `3061 selected, 3050 passed, 11 skipped, 0 failed`. `EXPECT_SKIPPED=11` holds exactly, and the floors are met. - **The sandbox-only `LC_CTYPE` failure above does not reproduce in this run.** It runs with `LANG=C.UTF-8`. - **#67 (T070) and #69 (T072) stack on this branch** and take this head in their own merge rounds. ## Fix round: a PostgreSQL scheme libpq would not read as a URI is refused (`c39d960e`) Copilot's review at `adeb6fed` noted that `postgresql:foo` was still accepted. [Answered on the PR](#60 (comment)). - `urlsplit` reads `postgresql:` without `//`, and any capitalized `PostgreSQL://` or `POSTGRES://`, as the PostgreSQL scheme. libpq reads none of them as a URI, and its refusal **repeats the whole value**, password included (measured). - The dialect gate now refuses a PostgreSQL scheme in any spelling but libpq's two (`postgresql://`, `postgres://`), naming the setting and never the value. - The new case covers 8 inputs. All fail against `adeb6fed` and pass here, and 5 mutants are killed. - Full suite: `3069 selected, 3058 passed, 11 skipped, 0 failed`. ## Resolved by ruling Brett ruled on the conflict Copilot's review found (openxFactory#656, on the claim thread for plan 034's T071, 2026-09-28), choosing **"Required only for migrate (Recommended)"**: - Keep both refusals: the non-PostgreSQL DSN, and one credential given in both settings. - Require `OPENDOX_MIGRATION_DATABASE_URL` only on the `migrate` path. - `serve` and `status` keep it OPTIONAL, and it is never defaulted from `OPENDOX_DATABASE_URL`. - When both are given, the collapse refusal still applies. This matches #1144 13.3's own text and `deploy/compose/docker-compose.yaml`'s separation (the `opendox` service never gets a migration DSN; `docs/runtime.md` § 3 never lists it as required) — **neither file needed a change**; both already documented the now-ruled behavior. The plan's "stops being optional" line is a holder-side correction, not part of this PR, and #1144's own wording is unchanged. Reworked in `b5296f9` per the ruling: the required→optional flip is reverted, the 27-site ripple it forced is reverted with it, and two new CLI-level tests (`test_serve_and_status_load_with_no_migration_dsn_configured`, `test_the_collapse_is_refused_through_the_served_workload_too`) prove the ruled shape end to end. [The original finding](#60 (comment)) and [my first reply](#60 (comment)) (flagging it for a ruling) are in the review thread, [now closed out citing the ruling](#60 (comment)) and marked resolved. ## Scope Touches: `src/opendox/runtime/config.py`, `tests_runtime/conftest.py`, `tests_runtime/test_api_endpoints.py`, `tests_runtime/test_migrations_apply.py`, `tests_runtime/test_runtime_cli.py`, `tests_runtime/test_runtime_surface.py`. Nothing else — no `conftest.py` at the repo root, no `pyproject.toml`, no workflow file, no `README.md`, no pin, no `deploy/`, no `docs/`. No `openspec/changes/` path, so no Rule 6 window applies. The holder's lander merges. T063 has landed (openxFactory#1218 → `a883bbf6`), so the holder un-drafts it. Arc: neutral-product-standalone-operability 🤖 Generated with [Claude Code](https://claude.com/claude-code) Arc: neutral-product-standalone-operability Lane: openxfactory-4 (openXfactory-4-openDox_extraction) Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>



What this is
A small follow-up to T055 (#59, landed as
fa140875). It takes the one finding Copilot's review of #59 at0c946f4eraised as "previously missed", after the last push that could take it.The finding, quoted
Copilot review overview on #59 at
0c946f4e(review5368674478), medium, "Avoid unstable second lookup during source path confinement",src/opendox/default_registry.py:444:It is real. #59 already closed the same pattern in
serve.py's/sourcearm (Copilot r4136585695 and r4136863569): that arm resolves the entry once and confines to that entry's own root withresolve_source_path(Path(root), rest).serve_project._resolved_listed_edit_entrywas the one caller left.What changed
src/opendox/serve_project.py:_resolved_listed_edit_entryresolves the entry ONCE and confines the path to THAT entry's own root through the registry seam's declaredresolve_within(read asprojection_seams.registry.current()inside the function). The listed-path check already read the entry in hand, and the editor is launched overentry.source_root, so the lookup, the validation and the confinement are now one entry. An entry with no root serves nothing, as before.resolve_sourceis on no seam's list (REGISTRY_CALLABLES), so a contributed registry is never asked for one. That is why the fix confines the entry in hand rather than adding an entry-based method to openDox's own registry: a host's registry would not carry it.src/opendox/default_registry.py:SnapshotRegistry.resolve_sourcekeeps its behaviour (one lookup, confined to that entry). Its docstring now says it is for a caller that holds only a pair, and why a caller that already holds an entry must not ask again by its pair.tests/test_edit_action_one_entry.py(new, 8 cases).No module-level proxy is bound in
serve_project.py:tests/test_projection_seams.py::test_no_proxy_over_a_seam_is_read_at_import_timepins the exact set of modules that bind one (serve.py,serve_workbench.py), and this PR does not edit that file.Every caller of the two-step path
git grep resolve_source -- srcatfa140875finds the definition and exactly one production caller,serve_project.py:111. The other registry lookups insrc/are one resolution each:serve.py_serve_snapshotand_serve_source(already one entry),serve_workbench.py(resolvethenresolve_within(entry.source_root, ...), three sites), andbranch_session.py(stamping an entry after a register, no confinement)._keyed_source'sregistry.get(*parsed)is a parse-time existence check whose result is a key, and_serve_sourcethen resolves that key once.The new test
test_no_module_asks_a_registry_for_a_path_by_a_pairholds that set empty, so a future caller has to be argued for.Evidence
Red at main, green after. The new module against
fa140875's sources (serve_project.pyanddefault_registry.pyas on main):With this PR:
8 passed. The three that pass at main are the control (a listed file opens with no refresh), confinement kept, and "no root serves nothing", which pin what must NOT change.The race is tested at the ROUTE, both ways, with a real server, a real
POST /actions/editand the console token. A registry whose key is replaced right after the route's firstresolve(as a refresh on another thread would) lands the replacement between the resolution and the confinement (asserted):200and an editor over a file that is not there. Now:404 document_unavailable, no editor.404). Now:200, and the editor is started over that entry's root.Mutants of the fix, all killed (the new module only,
serve_project.pyrestored after each):registry.resolve_source(repository, ref, path), i.e. main)Path(root) / path, no containment ruletest_the_entrys_own_root_still_confines_what_the_route_acceptsT056's module against this fix. Fetched #66's head
38761c76read-only into a scratch worktree, merged main (fa140875; the two add/add conflicts,default_registry.pyandtests/test_projection_seams.py, resolved to main's blobs, as T056's own diff does not touch either), cherry-picked this commit on top, and rantests/test_standalone_generate_path.py(its children run from that worktree'ssrcviaPYTHONPATH):That module's requests are
GET /source/notes-toolshed-inventory.md(200, byte-equal) andGET /source/.git/config(404). They go throughserve.py's/sourcearm (resolve_source_path), which #59 already moved offresolve_source, not throughserve_project, so this PR leaves them as they were. The module passes identically with and without this commit.Whole suite at
5666505b,LANG=C.UTF-8, run in the foreground with a throwawaypostgres:16andOPENDOX_TEST_DATABASE_URLset as CI sets it:#59's CI at
0c946f4ewasselected=2989 passed=2978 skipped=11. This PR adds 8 cases: 2997 selected, 2986 passed, 11 skipped.Overlap with open PRs
None.
gh pr diff --name-onlyon #60 to #69, read against each PR's own merge-base (#66 and #68 carry #59's commits, which listsdefault_registry.pyspuriously): their own diffs touch neitherserve_project.pynordefault_registry.py. #66's ownserve.pychange is one flushedprintnear line 2216, outside the/sourcearm.Review rounds
0471f8c7: "Approval recommended", no findings, 0 threads. The SonarCloud quality gate failed there on 4.9% duplication on new code (required at most 3%): the new test module carried a copy oftest_projection_seams.py's autouse isolation fixture andgithelper.5666505b: imports both instead of copying them (thetests/test_doxbench_*.pyprecedent), a test-only change. The autouse fixture still applies to all eight cases (eight SETUPs under--setup-show), the eight cases pass, and M1 to M5 are still killed.5666505b: "Approval recommended", no findings, 0 threads.validateand SonarCloud ("Quality Gate passed") green at5666505b.Arc: neutral-product-standalone-operability
Lane: openxfactory-4 (openXfactory-4-openDox_extraction)
🤖 Generated with Claude Code